The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Review	Updated (UTC)
daily-webapp	Ready	Preview	Jan 13, 2026 11:50am

1 Skipped Deployment

Project	Deployment	Review	Updated (UTC)
storybook	Ignored		Jan 13, 2026 11:50am

Claude finished @AmarTrebinjac's task in 1m 30s —— View job

Reviewing PR feedback

Tasks

• Read ProfileCompletionCard.tsx
• Read useProfileCompletionCard.ts hook
• Analyze review comments
• Provide feedback on each comment

Review Feedback

I've reviewed @rebelchris's comments and here's my analysis:

1. Lines 36-37, 42-43: Inline styles should be Tailwind

✅ AGREE - These should be converted to Tailwind utilities where possible.

• Line 37 (border: profileCompletionCardBorder): The border uses color-mix() with cabbage accent. While Tailwind doesn't have a direct utility for this complex color-mix, this could potentially use a border color class if we add it to the theme.
• Line 43 (background: profileCompletionButtonBg): Same situation with color-mix() for transparency.

However, I notice the codebase has a pattern of using backdrop-blur-3xl as a Tailwind class (line 164), so that part is already correct.

The question is: should these color-mix values be in the Tailwind config or as CSS variables? Looking at the existing pattern, it seems like the answer depends on reusability.

2. Line 40: Gradient should be in gradient file

✅ STRONGLY AGREE - This is an established pattern in the codebase.

Looking at packages/shared/src/styles/custom.ts, I can see there's already a dedicated file for custom colors and gradients. The file contains:

• briefCardBorder, briefCardBg, briefButtonBg (lines 12-18)
• bookmarkProviderListBg (line 9-10)
• Other similar custom styles

The profile completion card styles should follow this exact same pattern and be moved to custom.ts. This improves:

• Maintainability: One place to update colors
• Consistency: Same approach as brief card
• Discoverability: Other developers know where to find custom styles

Fix this →

3. Line 118: Too many calculations split between hook and component

🤔 PARTIALLY AGREE - There is some duplication, but it's not as bad as it might seem.

Looking at the code:

In the hook (useProfileCompletionCard.ts):

const isCompleted = (profileCompletion?.percentage ?? 100) === 100;
const isDismissed = isActionsFetched && checkHasCompleted(ActionType.ProfileCompletionCard);
const shouldEvaluate = isMyFeed && !isCompleted && hasNotDismissed;

In the component (ProfileCompletionCard.tsx:112-118):

const progress = profileCompletion?.percentage ?? 0;
const isCompleted = progress === 100;
const isDismissed = isActionsFetched && checkHasCompleted(ActionType.ProfileCompletionCard);
const shouldShow = profileCompletion && !isCompleted && firstIncompleteItem && !isDismissed;

The duplication is:

• ✅ isCompleted calculation - duplicated
• ✅ isDismissed calculation - duplicated

However, there's a reason: The hook is used in Feed.tsx to determine whether to show the profile completion card OR the brief card (lines 212-221). The component needs its own shouldShow logic because it has additional requirements (firstIncompleteItem must exist).

Possible improvements:

1. The component could receive isCompleted and isDismissed as props from the parent (Feed.tsx already calls the hook)
2. OR keep the current approach since the logic is simple and the duplication is minimal

I'd say this is a minor issue - the duplication is only 2 simple calculations, and having the component be self-contained has benefits for testing and reusability. But if we want to be strict about DRY principles, we could refactor.

Consider this refactor →

Summary

The most important fix is moving the custom styles to custom.ts to follow the established codebase pattern. The calculation duplication is minor and may not be worth refactoring given the current architecture.